Feature/deseng439: Created Timline Widget and merged with Content from Main #2347
Conversation
Add a change log to allow developers to log any changes they make to the project.
* Feature: update sample .env files * Remove old production .env file * Update DEVELOPMENT.md to reflect project state * Update CHANGELOG.md before PR * Link JIRA ticket # on relevant changes
* Made slug url case insensitive , Fixed bug with wrong query join for submission (#2321) * Made slug url case insensitive , Fixed bug with wrong query join for submission * removed a couple of if statements * CSV export made working for multipage wizard surveys (#2322) --------- Co-authored-by: saravanpa-aot <[email protected]>
…ame window/tab as opposed to a new one. (#2329)
* bugfix/deseng413: Upgraded BC-Sans font to newest version. * bugfix/deseng413: Small update to changelog for clarification.
* feature/deseng415: Added recording of date with feedback submission and displaying the data on admin side. * feature/deseng415: Fixed feedback schema, removed yup import, fixed change log date.
* bugfix/deseng429: Removed outdated service class. * bugfix/deseng429: Changed version and changelog to match deployments to gdx-main.
jareth-whitney
requested review from
Baelx,
NatSquared,
ShawnTurple,
kshapka-bcgov and
robrien-bcgov
| { | ||
| 'id': 8, | ||
| 'name': 'CAC Form', | ||
| 'description': 'Add a CAC Form to your project', |
There was a problem hiding this comment.
I imagine this example text should be changed? CAC forms are unrelated to timelines, right?
There was a problem hiding this comment.
So, in met-web, for the widget type ENUM, value 8 is reserved for CAC forms. Timeline is 9. When I looked in the DB, CAC Form was missing as a widget type, so I added it as number 8 and added Timeline as number 9. I thought it would be better to leave that path open, rather than remove all instances of CAC Form.
| return str(err), err.status_code | ||
|
|
||
|
|
||
| @cors_preflight('PATCH') |
There was a problem hiding this comment.
Not your issue but I don't think the API ever caches the preflight response in the client, right? We should implement that at some point - I'll create a ticket. Each options call is a round-trip request we could be saving.
There was a problem hiding this comment.
OK sounds good, so I think there's no action items for this PR related to this comment.
There was a problem hiding this comment.
Correct! No action item needed for the PR but I was wondering if you knew if the preflight responses were ever cached.
There was a problem hiding this comment.
No I don't know much about the preflight cache. It sounds like you already know more than me!
| return TimelineEventModel.update_timeline_event(timeline_event.id, event_data) | ||
|
|
||
| @staticmethod | ||
| def _create_timeline_event_model(timeline_id, event_data: dict): |
There was a problem hiding this comment.
I don't know if we need to have a separate method to build up the model data but if you want to keep it as a "private" class method then you could preface the name with 2 underscores as is convention.
There was a problem hiding this comment.
If you look at all of the other services, it seems like everything uses the single underscore convention. I'm not disagreeing that it should be updated, but perhaps that should be a separate ticket where someone changes all instances at once.
| events = db.relationship(TimelineEvent, backref='widget_timeline', lazy=True) | ||
|
|
||
| @classmethod | ||
| def get_timeline(cls, timeline_id) -> list[WidgetTimeline]: |
There was a problem hiding this comment.
There seems to be a pattern throughout the app of models only containing some CRUD methods. In looking through the models, some have all 4 methods (GET, PATCH, POST, DELETE) and sometimes half of those are handled in the Service class (including the corresponding DB operations).
I don't understand why that pattern is in place but I feel all CRUD operations should live inside the models. Was there a reason that you didn't include the POST operation besides just copying from another file? I remember you mentioned the DELETE operation is handled by the base Widget class, so I understand that.
I know we have pressure on us to get features done but I also feel the PO would understand us taking more time to make things more consistent and better in the code base.
There was a problem hiding this comment.
So anything that touches the db is defined in the model. Then the service just uses those methods from the model to do stuff. I was confused at first too, and so was Nat, but after looking at the logic, I now understand it. Sometimes a service method doesn't need to call a model method that touches the db, so nothing is defined in the model.
met-web/src/components/engagement/view/widgets/Timeline/TimelineWidgetView.tsx
Outdated
Show resolved
Hide resolved
met-web/src/components/engagement/view/widgets/Timeline/TimelineWidgetView.tsx
Outdated
Show resolved
Hide resolved
met-web/src/components/engagement/view/widgets/Timeline/TimelineWidgetView.tsx
Outdated
Show resolved
Hide resolved
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2347 +/- ##
==========================================
+ Coverage 68.56% 68.93% +0.37%
==========================================
Files 472 509 +37
Lines 15593 15989 +396
Branches 1193 1168 -25
==========================================
+ Hits 10691 11022 +331
- Misses 4693 4758 +65
Partials 209 209
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…m Main (#2347) * Add initial version of change log (#2318) Add a change log to allow developers to log any changes they make to the project. * Feature/update sample env files (#2320) * Feature: update sample .env files * Remove old production .env file * Update DEVELOPMENT.md to reflect project state * Update CHANGELOG.md before PR * Link JIRA ticket # on relevant changes * Bring bugfixes from main into gdx-dev (#2328) * Made slug url case insensitive , Fixed bug with wrong query join for submission (#2321) * Made slug url case insensitive , Fixed bug with wrong query join for submission * removed a couple of if statements * CSV export made working for multipage wizard surveys (#2322) --------- Co-authored-by: saravanpa-aot <[email protected]> * bugfix/deseng421: Changed engagement links so that they open in the same window/tab as opposed to a new one. (#2329) * Bugfix/deseng413 (#2330) * bugfix/deseng413: Upgraded BC-Sans font to newest version. * bugfix/deseng413: Small update to changelog for clarification. * Feature/deseng415 (#2334) * feature/deseng415: Added recording of date with feedback submission and displaying the data on admin side. * feature/deseng415: Fixed feedback schema, removed yup import, fixed change log date. * bugfix/deseng429: Removed outdated service class. (#2337) * bugfix/deseng429: Removed outdated service class. * bugfix/deseng429: Changed version and changelog to match deployments to gdx-main. * feature/deseng439: Finished the Timeline Widget and linted. * feature/deseng439: Updated changelog before merge. * feature/deseng439: Made revisions as per Alex and added sorting to React components. --------- Co-authored-by: Nat² <[email protected]> Co-authored-by: saravanpa-aot <[email protected]>
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the met-public license (Apache 2.0).